Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: refactor the old goroutine execution sweep #18361

Merged
merged 1 commit into from
Mar 21, 2023

Conversation

chlins
Copy link
Member

@chlins chlins commented Mar 16, 2023

  1. Delete the old goroutine execution sweeper when create execution(in the case of high concurrency can cause goroutine backlogs, affect the performance of core)
  2. Introduce the new way to sweep executions, a global scheduled job will take the work.

Thank you for contributing to Harbor!

Comprehensive Summary of your change

Issue being fixed

Fixes #(issue)

Please indicate you've done the following:

  • Well Written Title and Summary of the PR
  • Label the PR as needed. "release-note/ignore-for-release, release-note/new-feature, release-note/update, release-note/enhancement, release-note/community, release-note/breaking-change, release-note/docs, release-note/infra, release-note/deprecation"
  • Accepted the DCO. Commits without the DCO will delay acceptance.
  • Made sure tests are passing and test coverage is added if needed.
  • Considered the docs impact and opened a new docs issue or PR with docs changes if needed in website repository.

@chlins chlins requested a review from a team as a code owner March 16, 2023 04:48
@codecov
Copy link

codecov bot commented Mar 16, 2023

Codecov Report

Merging #18361 (d1eb8ca) into main (2b3f178) will increase coverage by 3.13%.
The diff coverage is 37.58%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #18361      +/-   ##
==========================================
+ Coverage   67.41%   70.55%   +3.13%     
==========================================
  Files         976      747     -229     
  Lines      106493    93634   -12859     
  Branches     2662        0    -2662     
==========================================
- Hits        71794    66061    -5733     
+ Misses      30840    24004    -6836     
+ Partials     3859     3569     -290     
Flag Coverage Δ
unittests 70.55% <37.58%> (+3.13%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/controller/gc/controller.go 72.26% <ø> (-0.60%) ⬇️
src/controller/p2p/preheat/enforcer.go 54.84% <ø> (-0.38%) ⬇️
src/controller/replication/execution.go 69.41% <ø> (-0.54%) ⬇️
src/controller/retention/controller.go 47.63% <ø> (-0.67%) ⬇️
src/controller/scan/callback.go 82.35% <0.00%> (ø)
src/controller/scandataexport/execution.go 87.83% <ø> (-0.17%) ⬇️
src/controller/systemartifact/execution.go 72.52% <ø> (-0.60%) ⬇️
src/jobservice/job/known_jobs.go 0.00% <0.00%> (ø)
src/pkg/task/execution.go 67.43% <ø> (+5.97%) ⬆️
src/controller/task/sweep.go 10.76% <10.76%> (ø)
... and 6 more

... and 238 files with indirect coverage changes

@chlins chlins assigned wy65701436 and unassigned zyyw and MinerYang Mar 16, 2023
@chlins chlins added target/2.8.0 release-note/update Update or Fix kind/refactor The issue is to track the effort of refactory. labels Mar 16, 2023
@chlins chlins force-pushed the refactor/execution-sweep branch 4 times, most recently from 010d953 to 6931144 Compare March 16, 2023 09:21
src/pkg/task/sweep_job.go Outdated Show resolved Hide resolved
@chlins chlins force-pushed the refactor/execution-sweep branch from 6931144 to 5076e25 Compare March 19, 2023 07:28
Copy link
Contributor

@stonezdj stonezdj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@chlins chlins force-pushed the refactor/execution-sweep branch 2 times, most recently from 5cfaa59 to 9334456 Compare March 20, 2023 06:31
Copy link
Contributor

@wy65701436 wy65701436 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

…ion sweep job

1. Delete the old goroutine execution sweeper when create execution.(in the case of high concurrency can cause goroutine backlogs, affect the performance of core)
2. Introduce the new way to sweep executions, a global scheduled job will take the work.

Signed-off-by: chlins <[email protected]>
@chlins chlins force-pushed the refactor/execution-sweep branch from 9334456 to d1eb8ca Compare March 20, 2023 11:00
@chlins chlins merged commit f21b148 into goharbor:main Mar 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/gc kind/refactor The issue is to track the effort of refactory. release-note/update Update or Fix target/2.8.0
Projects
Status: Completed
Development

Successfully merging this pull request may close these issues.

6 participants